-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JN-1486] Referral source + skip pre-enroll + pre-fill pre-enroll #1280
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... I'm not sure about adding "referralSource" as a property on Enrollee -- partly because I'm not sure it belongs there, and partly because referral source might get complicated and might be an object, or multiple fields (e.g. referring site + referring person). Think about adding it to PreEnrollmentResponse instead, and have it be either part of the fullData, or a dedicated answer like "qualified"
import { useSearchParams } from 'react-router-dom' | ||
import { Answer } from '@juniper/ui-core' | ||
|
||
export function useEnrollmentParams() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a great use of hooks
@@ -76,6 +77,7 @@ export default function UserProvider({ children }: { children: React.ReactNode } | |||
const [loginState, setLoginState] = useState<LoginResult | null>(null) | |||
const [isLoading, setIsLoading] = useState(true) | |||
const auth = useAuth() | |||
useEnrollmentParams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this?
@@ -54,7 +54,6 @@ const ParticipantTermsOfUsePage = lazy(() => import('terms/ParticipantTermsOfUse | |||
const ScrollToTop = () => { | |||
const location = useLocation() | |||
useEffect(() => { | |||
// @ts-expect-error TS thinks "instant" isn't a valid scroll behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDE didn't like this anymore, probably thanks to a dependency upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great! Super smooth. I like the UX for showing the referral source on the pre-enroll, but I do wonder if we should also show the referral source on the main enrollee page
@@ -28,4 +28,5 @@ public class PreEnrollmentResponse extends BaseEntity { | |||
private String fullData; | |||
@Builder.Default | |||
private boolean qualified = false; // whether or not the responses meet the criteria for eligibility. | |||
private String referralSource; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we going to be using PreEnrollmentResponse forever? I was under the impression this was on its way out in favor of making pre-enrollments just regular surveys, but we haven't done that work yet. Right now, pre-enrolls generate normal answers as well as a PreEnrollmentResponse object.
useEffect(() => { | ||
captureEnrollmentParams() | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool! I like how this works, but do you mind adding a comment explaining it? I was a bit confused and didn't know what this did at first.
@@ -33,9 +34,13 @@ export default function PreEnrollView({ enrollContext, survey }: | |||
const navigate = useNavigate() | |||
// for now, we assume all pre-screeners are a single page | |||
const pager = { pageNumber: 0, updatePageNumber: () => 0 } | |||
|
|||
const { preFilledAnswers, referralSource, clearStoredEnrollmentParams } = useEnrollmentParams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit wary of this not being persisted in the URL. I think most of our redirects right now don't preserve query params, so it makes sense, but I do feel like it could make some situations harder to debug (+ harder to track with mixpanel). It works well as-is, though, so I'm happy to 👍 !
0f1346a
to
224667b
Compare
|
DESCRIPTION (include screenshots, and mobile screenshots for participant UX)
Adds three new bits of functionality for enrollment:
skipPreEnroll=true
in the query paramsAdmin UX. Is this too prominent?
TO TEST: (simple manual steps for confirming core behavior -- used for pre-release checks)
Restart admin api and participant api
Test prefilled answers by going to:
Click register
Confirm the first two answers are filled out already with the value "yes"
Test skip pre-enroll by going to
https://sandbox.ourhealth.localhost:3001/?skipPreEnroll=true
Confirm you're taken directly to the account creation screen and registration can be completed
Test referring site by going to
https://sandbox.ourhealth.localhost:3001/?referralSource={%22referringSite%22:%22buildclinical.com%22}
Register for an account
Go to the admin tool and confirm you see the referral information on the new participant's pre enrollment view